phptek / silverstripe-sentry

Flexible Sentry compatible bug aggregation client for Silverstripe applications.
BSD 3-Clause "New" or "Revised" License
12 stars 22 forks source link

Minimum Error Log and other fixes #49

Closed halles closed 4 years ago

halles commented 4 years ago
halles commented 4 years ago

Hey @phptek, after a few months with this on the table, I've finally managed to sit down and solve the issue with the min log level. Turns out the Logger config would just set any message to a certain level rather than filtering out unwanted noise. Let us know if you got comments or want changes.

Cheers!

phptek commented 4 years ago

Wow! Thank you! My only comments are: Make sure that SENTRY_DSN still trumps Config if set, and to ensure you add explicit return values to all newly added methods or newly modified methods. That's all :-)

halles commented 4 years ago

Hi! That part of the config has not been touched. The DSN still gets set in the Adaptor, but the log_level is a parameter that belongs in the Monolog configuration, and set in the SentryHandler itself. I'd argue that if we want to support multiple Sentry Handlers (as Monolog does support more than one handler) this requires a bigger refactoring.

Also, I haven't created any new methods or modified any that would require changes in that respect.

phptek commented 4 years ago

Also, I haven't created any new methods or modified any that would require changes in that respect.

Yeah, my bad. Shows how much detail one can go into on a poxy phone. Thanks again for your help!

phptek commented 4 years ago

Just created 3.0.4 for you too.

halles commented 4 years ago

Awesome. Thanks!