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.

Sign up for GitHub

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

Jump to bottom

Add Sentry BreadcrumbHandler support #489

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

Add Sentry BreadcrumbHandler support #489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,9 @@
* - hub_id: Sentry hub custom service id (optional)
* - [fill_extra_context]: bool, defaults to false
*
* - sentry_breadcrumb:
* - sentry_handler: the sentry handler's name
*
* - newrelic:
* - [level]: level name or int value, defaults to DEBUG
* - [bubble]: bool, defaults to true
Expand Down Expand Up @@ -534,7 +537,7 @@ public function getConfigTreeBuilder(): TreeBuilder
->scalarNode('max_level')->defaultValue('EMERGENCY')->end() // filter
->scalarNode('buffer_size')->defaultValue(0)->end() // fingers_crossed and buffer
->booleanNode('flush_on_overflow')->defaultFalse()->end() // buffer
->scalarNode('handler')->end() // fingers_crossed and buffer
->scalarNode('handler')->end() // fingers_crossed, buffer, filter, deduplication, sampling
->scalarNode('url')->end() // cube
->scalarNode('exchange')->end() // amqp
->scalarNode('exchange_name')->defaultValue('log')->end() // amqp
Expand Down Expand Up @@ -584,6 +587,7 @@ public function getConfigTreeBuilder(): TreeBuilder
->booleanNode('persistent')->end() // socket_handler
->scalarNode('dsn')->end() // raven_handler, sentry_handler
->scalarNode('hub_id')->defaultNull()->end() // sentry_handler
->scalarNode('sentry_handler')->defaultNull()->end() // sentry_breadcrumb
->scalarNode('client_id')->defaultNull()->end() // raven_handler, sentry_handler
->scalarNode('auto_log_stacks')->defaultFalse()->end() // raven_handler
->scalarNode('release')->defaultNull()->end() // raven_handler, sentry_handler
Expand Down Expand Up @@ -658,8 +662,8 @@ public function getConfigTreeBuilder(): TreeBuilder
->thenInvalid('Service handlers can not have a formatter configured in the bundle, you must reconfigure the service itself instead')
->end()
->validate()
->ifTrue(function ($v) { return ('fingers_crossed' === $v['type'] || 'buffer' === $v['type'] || 'filter' === $v['type'] || 'sampling' === $v['type']) && empty($v['handler']); })
->thenInvalid('The handler has to be specified to use a FingersCrossedHandler or BufferHandler or FilterHandler or SamplingHandler')
->ifTrue(function ($v) { return ('fingers_crossed' === $v['type'] || 'buffer' === $v['type'] || 'filter' === $v['type'] || 'deduplication' === $v['type'] || 'sampling' === $v['type']) && empty($v['handler']); })
->thenInvalid('The handler has to be specified to use a FingersCrossedHandler, BufferHandler, FilterHandler, DeduplicationHandler or SamplingHandler')
->end()
->validate()
->ifTrue(function ($v) { return 'fingers_crossed' === $v['type'] && !empty($v['excluded_404s']) && !empty($v['activation_strategy']); })
Expand Down Expand Up @@ -725,6 +729,10 @@ public function getConfigTreeBuilder(): TreeBuilder
->ifTrue(function ($v) { return 'sentry' === $v['type'] && null !== $v['hub_id'] && null !== $v['client_id']; })
->thenInvalid('You can not use both a hub_id and a client_id in a Sentry handler')
->end()
->validate()
->ifTrue(function ($v) { return 'sentry_breadcrumb' === $v['type'] && empty($v['sentry_handler']); })
->thenInvalid('The sentry_handler has to be specified to use a Sentry BreadcrumbHandler')
->end()
->validate()
->ifTrue(function ($v) { return 'hipchat' === $v['type'] && (empty($v['token']) || empty($v['room'])); })
->thenInvalid('The token and room have to be specified to use a HipChatHandler')
Expand Down
29 changes: 24 additions & 5 deletions DependencyInjection/MonologExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use Symfony\Bridge\Monolog\Processor\TokenProcessor;
use Symfony\Bridge\Monolog\Processor\WebProcessor;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\Argument\AbstractArgument;
use Symfony\Component\DependencyInjection\Argument\BoundArgument;
use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\ContainerBuilder;
Expand All @@ -47,6 +48,8 @@ class MonologExtension extends Extension
{
private $nestedHandlers = [];

private $sentryBreadcrumbHandlers = [];

private $swiftMailerHandlers = [];

/**
Expand Down Expand Up @@ -79,6 +82,11 @@ public function load(array $configs, ContainerBuilder $container)
];
}

foreach ($this->sentryBreadcrumbHandlers as $sentryBreadcrumbHandlerId => $sentryHandlerId) {
$hubId = (string) $container->getDefinition($sentryHandlerId)->getArgument(0);
$container->getDefinition($sentryBreadcrumbHandlerId)->replaceArgument(0, new Reference($hubId));
}

$container->setParameter(
'monolog.swift_mailer.handlers',
$this->swiftMailerHandlers
Expand Down Expand Up @@ -724,7 +732,7 @@ private function buildHandler(ContainerBuilder $container, $name, array $handler

case 'sentry':
if (null !== $handler['hub_id']) {
$hub = new Reference($handler['hub_id']);
$hubId = $handler['hub_id'];
} else {
if (null !== $handler['client_id']) {
$clientId = $handler['client_id'];
Expand Down Expand Up @@ -755,24 +763,34 @@ private function buildHandler(ContainerBuilder $container, $name, array $handler
}
}

$hub = new Definition(
$hubId = \sprintf('monolog.handler.%s.hub', $name);
$hub = $container->setDefinition($hubId, new Definition(
'Sentry\\State\\Hub',
[new Reference($clientId)]
);
$container->setDefinition(\sprintf('monolog.handler.%s.hub', $name), $hub);
));
Comment on lines -758 to +770
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This never worked correctly. The monolog.handler.%s.hub service was registered but never actually used. Instead, the entire definition was passed as an argument, resulting in the creation of two separate services: the unused monolog.handler.%s.hub service and an anonymous service that was passed to the handler.


// can't set the hub to the current hub, getting into a recursion otherwise...
// $hub->addMethodCall('setCurrent', array($hub));
}

$definition->setArguments([
$hub,
new Reference($hubId),
$handler['level'],
$handler['bubble'],
$handler['fill_extra_context'],
]);
break;

case 'sentry_breadcrumb':
$this->sentryBreadcrumbHandlers[$handlerId] = $this->getHandlerId($handler['sentry_handler']);

$definition->setArguments([
new AbstractArgument('Sentry hub id'),
$handler['level'],
$handler['bubble'],
]);
break;

case 'raven':
if (null !== $handler['client_id']) {
$clientId = $handler['client_id'];
Expand Down Expand Up @@ -977,6 +995,7 @@ private function getHandlerClassByType($handlerType)
'pushover' => 'Monolog\Handler\PushoverHandler',
'raven' => 'Monolog\Handler\RavenHandler',
'sentry' => 'Sentry\Monolog\Handler',
'sentry_breadcrumb' => 'Sentry\Monolog\BreadcrumbHandler',
'newrelic' => 'Monolog\Handler\NewRelicHandler',
'hipchat' => 'Monolog\Handler\HipChatHandler',
'slack' => 'Monolog\Handler\SlackHandler',
Expand Down
65 changes: 57 additions & 8 deletions Tests/DependencyInjection/MonologExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,12 +371,10 @@ public function testRavenHandlerWhenAClientIsSpecified()

public function testSentryHandlerWhenConfigurationIsWrong()
{
try {
$this->getContainer([['handlers' => ['sentry' => ['type' => 'sentry']]]]);
$this->fail();
} catch (InvalidConfigurationException $e) {
$this->assertStringContainsString('DSN', $e->getMessage());
}
$this->expectException(InvalidConfigurationException::class);
$this->expectExceptionMessage('The DSN has to be specified to use Sentry\'s handler');

$this->getContainer([['handlers' => ['sentry' => ['type' => 'sentry']]]]);
}

public function testSentryHandlerWhenADSNIsSpecified()
Expand Down Expand Up @@ -426,7 +424,10 @@ public function testSentryHandlerWhenADSNAndAClientAreSpecified()
$this->assertDICDefinitionMethodCallAt(1, $logger, 'pushHandler', [new Reference('monolog.handler.sentry')]);

$handler = $container->getDefinition('monolog.handler.sentry');
$this->assertDICConstructorArguments($handler->getArguments()[0], [new Reference('sentry.client')]);
$this->assertDICConstructorArguments($handler, [new Reference('monolog.handler.sentry.hub'), 'DEBUG', true, false]);

$hub = $container->getDefinition($handler->getArguments()[0]);
$this->assertDICConstructorArguments($hub, [new Reference('sentry.client')]);
}

public function testSentryHandlerWhenAClientIsSpecified()
Expand All @@ -452,7 +453,10 @@ public function testSentryHandlerWhenAClientIsSpecified()
$this->assertDICDefinitionMethodCallAt(1, $logger, 'pushHandler', [new Reference('monolog.handler.sentry')]);

$handler = $container->getDefinition('monolog.handler.sentry');
$this->assertDICConstructorArguments($handler->getArguments()[0], [new Reference('sentry.client')]);
$this->assertDICConstructorArguments($handler, [new Reference('monolog.handler.sentry.hub'), 'DEBUG', true, false]);

$hub = $container->getDefinition($handler->getArguments()[0]);
$this->assertDICConstructorArguments($hub, [new Reference('sentry.client')]);
}

public function testSentryHandlerWhenAHubIsSpecified()
Expand Down Expand Up @@ -501,6 +505,51 @@ public function testSentryHandlerWhenAHubAndAClientAreSpecified()
);
}

public function testSentryBreadcrumbHandlerWhenConfigurationIsWrong()
{
$this->expectException(InvalidConfigurationException::class);
$this->expectExceptionMessage('The sentry_handler has to be specified to use a Sentry BreadcrumbHandler');

$this->getContainer([['handlers' => ['sentry_breadcrumb' => ['type' => 'sentry_breadcrumb']]]]);
}

/**
* @testWith [{"dsn": "http://a:b@c.com:9000/1"}, "monolog.handler.sentry.hub"]
* [{"hub_id": "sentry.hub"}, "sentry.hub"]
*/
public function testSentryBreadcrumbHandlerWhenConfigurationIsOk(array $config, string $hubId)
{
$container = $this->getContainer(
[
[
'handlers' => [
'sentry_breadcrumb' => ['type' => 'sentry_breadcrumb', 'sentry_handler' => 'sentry'],
'sentry' => ['type' => 'sentry'] + $config,
],
],
],
[
'sentry.hub' => new Definition(\Sentry\State\HubInterface::class),
]
);

$this->assertTrue($container->hasDefinition('monolog.logger'));
$this->assertTrue($container->hasDefinition('monolog.handler.sentry'));
$this->assertTrue($container->hasDefinition('monolog.handler.sentry_breadcrumb'));

$logger = $container->getDefinition('monolog.logger');
$this->assertDICDefinitionMethodCallAt(0, $logger, 'useMicrosecondTimestamps', ['%monolog.use_microseconds%']);
$this->assertDICDefinitionMethodCallAt(1, $logger, 'pushHandler', [new Reference('monolog.handler.sentry')]);
$this->assertDICDefinitionMethodCallAt(2, $logger, 'pushHandler', [new Reference('monolog.handler.sentry_breadcrumb')]);

$handler = $container->getDefinition('monolog.handler.sentry_breadcrumb');
$this->assertDICDefinitionClass($handler, 'Sentry\Monolog\BreadcrumbHandler');
$this->assertDICConstructorArguments($handler, [new Reference($hubId), 'DEBUG', true]);

$sentry = $container->getDefinition('monolog.handler.sentry');
$this->assertSame((string) $sentry->getArgument(0), (string) $handler->getArgument(0));
}

public function testLogglyHandler()
{
$token = '026308d8-2b63-4225-8fe9-e01294b6e472';
Expand Down
Loading