Skip to content

Commit

Permalink
feature #394 Add Sentry Hub instance in DI container to resolve event…
Browse files Browse the repository at this point in the history
… context issue (dedpikhto)

This PR was merged into the 3.x-dev branch.

Discussion
----------

Add Sentry Hub instance in DI container to resolve event context issue

Fixing Symfony issue with different Hub's while decorating default Sentry handler for extended event context.

Common way to extend log event context is to decorate Sentry\Monolog\Handler:
```php
final class SentryContextHandlerDecorator extends AbstractProcessingHandler
{
    private Handler $handler;

    public function __construct(Handler $handler)
    {
        $this->handler = $handler;

        parent::__construct($handler->getLevel(), $handler->getBubble());
    }

    public function handle(array $record): bool
    {
        $result = false;

        withScope(function (Scope $scope) use ($record, &$result): void {
            if (isset($record['context']) && is_array($record['context']) && count($record['context']) > 0) {
                $scope->setContext('context', $record['context']);
            }

            $result = $this->handler->handle($record);
        });

        return $result;
    }
}
```

But if you do so with Symfony DI, you will get two instances of Hub: one from DI container, and one with SentrySdk in withScope(...) call. Changes to one Hub's scope won't affect second's hub scope.

This PR adds ability to use shared Hub in custom decorators:
```php
App\Logging\Handler\SentryContextHandlerDecorator:
    decorates: monolog.handler.sentry
    arguments:
        - '@app\Logging\Handler\SentryContextHandlerDecorator.inner'
        - '@monolog.handler.sentry.hub'
```
where `sentry` in `monolog.handler.sentry.hub` is name of handler.

Decorator with shared Hub instance:
```php
final class SentryContextHandlerDecorator extends AbstractProcessingHandler
{
    private Handler $handler;
    private Hub $hub;

    public function __construct(Handler $handler, Hub $hub)
    {
        $this->handler = $handler;
        $this->hub = $hub;

        parent::__construct($handler->getLevel(), $handler->getBubble());
    }

    public function handle(array $record): bool
    {
        $result = false;

        $this->hub->withScope(function (Scope $scope) use ($record, &$result): void {
            if (isset($record['context']) && is_array($record['context']) && count($record['context']) > 0) {
                $scope->setContext('context', $record['context']);
            }

            $result = $this->handler->handle($record);
        });

        return $result;
    }
}
```

More comments about problem: getsentry/sentry-php#848 (comment)

Commits
-------

8a77eb9 Add Sentry Hub instance in DI container to resolve event context issue
  • Loading branch information
jderusse committed Mar 21, 2021
2 parents 888b54b + 8a77eb9 commit 6ba90b9
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 0 deletions.
1 change: 1 addition & 0 deletions DependencyInjection/MonologExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,7 @@ private function buildHandler(ContainerBuilder $container, $name, array $handler
'Sentry\\State\\Hub',
[new Reference($clientId)]
);
$container->setDefinition(sprintf('monolog.handler.%s.hub', $name), $hub);

// can't set the hub to the current hub, getting into a recursion otherwise...
//$hub->addMethodCall('setCurrent', array($hub));
Expand Down
5 changes: 5 additions & 0 deletions Tests/DependencyInjection/MonologExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -393,13 +393,18 @@ public function testSentryHandlerWhenADSNIsSpecified()
]]]]);
$this->assertTrue($container->hasDefinition('monolog.logger'));
$this->assertTrue($container->hasDefinition('monolog.handler.sentry'));
$this->assertTrue($container->hasDefinition('monolog.handler.sentry.hub'));

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

$handler = $container->getDefinition('monolog.handler.sentry');
$this->assertDICDefinitionClass($handler, 'Sentry\Monolog\Handler');

$hub = $container->getDefinition('monolog.handler.sentry.hub');
$this->assertDICDefinitionClass($hub, 'Sentry\State\Hub');
$this->assertDICConstructorArguments($hub, [new Reference('monolog.sentry.client.'.sha1($dsn))]);
}

public function testSentryHandlerWhenADSNAndAClientAreSpecified()
Expand Down

0 comments on commit 6ba90b9

Please sign in to comment.