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

When using Monolog\Handler informations added by sentry listeners are missing #250

Closed
JanMikes opened this issue Sep 28, 2019 · 7 comments
Closed
Milestone

Comments

@JanMikes
Copy link
Contributor

Hi, there is something very weird going on.

Here is my config

services:
    Sentry\Monolog\Handler:
        arguments:
            $hub: '@Sentry\State\HubInterface'
            $level: 'error'

monolog:
    handlers:
        sentry:
            type: service
            id: Sentry\Monolog\Handler
            level: notice

But when i log, etc $this->logger->critical(), event in sentry does not contain extra informations added by Sentry\SentryBundle\EventListener\RequestListener.

By debugging i found out there are 2 different instances of HubInterface. 1 lives in our DI container and second on is in static SentrySdk::$currentHub property.

What RequestListener does, is only saving this to the static hub, meanwhile in monolog handler, the instance from DI is wired:
Screenshot 2019-09-28 21 05 07

On a screenshot you can see these are 2 different instances of hubs, one contains client (created and autowired by DI) the other does not.

I am thinking, if there is not missing SentrySdk::setCurrentHub() with the hub created by DI?

Maybe i am just missing something, but this causes many confusions for me.

This might be related to #247

@JanMikes
Copy link
Contributor Author

Basically creating listener like this, before Sentry's one fixes the issue:

public function __construct(HubInterface $hub) {
        $this->hub = $hub; 
    }

    public function onKernelRequest(GetResponseEvent $event): void
    {
        SentryBundle::setCurrentHub($this->hub);
    }

@JanMikes
Copy link
Contributor Author

JanMikes commented Sep 28, 2019

Overall there is missing info about how to use sentry in Symfony applications - should i use \Sentry\captureException(); or inject/autowire HubInterface and then $this->hub->captureException() as both results in different logs in sentry as described in the issue.

I consider myself static global functions as antipattern as it is hidden dependency and much harder to mock in tests.

@Jean85
Copy link
Collaborator

Jean85 commented Sep 29, 2019

That's exactly what we discussed when we had to implement the Unified APIs in sentry/sentry.

I agree that there should be only one hub in the app if the user doesn't create one itself. We should treat this as a bug and try to not create a duplicated hub as a side effect.

@Jean85
Copy link
Collaborator

Jean85 commented Sep 30, 2019

I think this bug was introduced by #244, in particular by this removal: 8d90388#diff-09af2b221d2475b568160a53310abe2aL20-L22

@Jean85
Copy link
Collaborator

Jean85 commented Sep 30, 2019

@JanMikes can you tell me if #253 fixes the issue?

@JanMikes
Copy link
Contributor Author

Yes, great job! I can confirm it works.

I have added $this->uniqId = uniqid(); to Sentry\Client to verify and this is result:
Screenshot 2019-09-30 12 22 14

@Jean85
Copy link
Collaborator

Jean85 commented Sep 30, 2019

Perfect, thank you.

@Jean85 Jean85 added this to the 3.2 milestone Sep 30, 2019
@Jean85 Jean85 mentioned this issue Sep 30, 2019
@Jean85 Jean85 closed this as completed Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants