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

Allow to disable ErrorListener completely #232

Closed
B-Galati opened this issue Aug 21, 2019 · 6 comments · Fixed by #247
Closed

Allow to disable ErrorListener completely #232

B-Galati opened this issue Aug 21, 2019 · 6 comments · Fixed by #247
Milestone

Comments

@B-Galati
Copy link

Hello,

In the setup there is an optional step with monolog setup. But having both Monolog and the ErrorListener would produce duplicates in Sentry server no?

So IMHO it would be great to add option to disable the ErrorListener and rely entirely on Monolog to report errors to Sentry if one is willing to do so.

WDYT?

@Jean85
Copy link
Collaborator

Jean85 commented Aug 24, 2019

Wouldn't make more sense to not use this bundle entirely in that case, and just require the base Sentry SDK to work with Monolog?

@B-Galati
Copy link
Author

Wouldn't make more sense to not use this bundle entirely in that case, and just require the base Sentry SDK to work with Monolog?

Sure it's possible but it's simpler to use this bundle that configures everything for us with the extra data provided by the listeners.

Also, Monolog config is documented in the README (as optional) so it looks to be a valid use case for the bundle.

@Jean85
Copy link
Collaborator

Jean85 commented Aug 27, 2019

The config would live in a completely different instance of the Hub/Client, wouldn't it? IHMO it doesn't interoperate well..

@B-Galati
Copy link
Author

@Jean85 Yeap the goal is to use the service Sentry\State\HubInterface created by the bundle in monolog config + take advantage of the setup of the bundle (pre-configured paths, data, tags etc.).

@Jean85
Copy link
Collaborator

Jean85 commented Sep 26, 2019

I'm gradually changing my mind. #247 seems to tackle well this issue, and I would like to add a breadcrumb Monolog handler in the future.

@B-Galati
Copy link
Author

Let me know if you need anything!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants