monolog-bundle icon indicating copy to clipboard operation
monolog-bundle copied to clipboard

Disable sentry's default integrations when using it

Open Taluu opened this issue 6 years ago • 13 comments

Monolog already does it, as the request integration that comes with the default configuration of sentry.

Disabling it fixes it. Maybe we should add a marker that says we could activate it ? I doubt the interest but I could be wrong.

poke @B-Galati

Taluu avatar Aug 14 '19 16:08 Taluu

In fact it would be good to keep Request Integration enabled as it gives a lot of useful information.

B-Galati avatar Aug 14 '19 20:08 B-Galati

Thought about it, but as there's already the request processor on monolog, didn't think it'd be useful : https://github.com/Seldaek/monolog/blob/master/src/Monolog/Processor/WebProcessor.php

Taluu avatar Aug 14 '19 21:08 Taluu

That processor is cool but I think it is doing less than Sentry Request Integration. Also, the goal of the processor is to add information on every single log while Request Integration will add these informations on each captured event which makes more sense for a tool like Sentry where log context is less readable. Last thing, it's possible that the SentryHandler will not support the "extra" key from log context in future releases. Should be major one but still interesting to know.

B-Galati avatar Aug 14 '19 21:08 B-Galati

Last thing, it's possible that the SentryHandler will not support the "extra" key from log context in future releases.

Source ?

Anyway, I managed to readd the request integration. poke @B-Galati @lyrixx

Taluu avatar Aug 19 '19 15:08 Taluu

Last thing, it's possible that the SentryHandler will not support the "extra" key from log context in future releases.

Source ?

https://github.com/getsentry/sentry-python/pull/457#issuecomment-519591414 -> that links to a comment in sentry-php.

B-Galati avatar Aug 20 '19 05:08 B-Galati

IMHO it would simpler to manage with a factory class that handles all cases.

Like this one: https://github.com/B-Galati/monolog-sentry-handler/blob/master/doc/guide-symfony.md#step-1-configure-sentry-hub

B-Galati avatar Aug 20 '19 06:08 B-Galati

Also, It is needed to set the global Hub by calling Hub::setCurrent():

// A global HubInterface must be set otherwise some feature provided by the SDK does not work as they rely on this global state
Hub::setCurrent(new Hub($client));

B-Galati avatar Aug 20 '19 06:08 B-Galati

@Taluu needs to be rebased from master due to short array clean up :+1:

inverse avatar Sep 02 '19 21:09 inverse

Done. I'll try to find a way to set the global hub state, even though it's repulsive (through a factory or a configurator, probably)

Taluu avatar Sep 03 '19 07:09 Taluu

Hello what is the status of this PR? why @lyrixx asked for not merging it?

jderusse avatar Mar 15 '21 07:03 jderusse

IIRC, it was because sentry relies on a global state, preventing from really having multiple handlers. I can't remember having made any progress, but I think maybe they had some on their side.

We will need to check with sentry's sdk.

Taluu avatar Mar 15 '21 10:03 Taluu

Unfortunately, I can not remember. Sorry. BTW, je sentry handler that is integrated with monolog is really far from perfect.

In my application, I end up with a full rewrite + a basic "service" handler

lyrixx avatar Mar 15 '21 13:03 lyrixx

And since then they introduced a new major version of the SDK.

B-Galati avatar Mar 15 '21 16:03 B-Galati