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

missing consideration of the set log level #3

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

SeifertDaniel
Copy link

no matter which log levels are set in the settings, Sentry logs all levels

@alfredbez
Copy link
Contributor

Hello @SeifertDaniel, thanks for the PR! I created oxpsSentryLogLevel to make it possible to send log entries (from the Monolog-Logger which writes to oxideshop.log) to sentry, so I guess this was a bit misleading. It is not implemented yet, so all levels are sent to Sentry. The idea behind this is to set it to warning for example to see all warning log entries in Sentry and be able to configure alerts etc there.

I think creating another module setting like PHP Error Level would be the best solution. What do you think?

@SeifertDaniel
Copy link
Author

Ahh ok, that makes sense. I'll change it.

@alfredbez
Copy link
Contributor

@SeifertDaniel I guess this is ready for review, is that right?

@SeifertDaniel
Copy link
Author

Sure.

@alfredbez alfredbez self-assigned this Feb 3, 2020
@alfredbez
Copy link
Contributor

btw there is already an issue for the monolog thing: #2

@pvanhemmen
Copy link

Hey @alfredbez would it be possible to merge this request? Or is this project abandoned?

@alfredbez
Copy link
Contributor

Hello there, I'm not using OXID anymore, but since this lives in the OXIDProjects organization anyone with write access can maintain that. Maybe ping @kermie if you have questions about that process.

@alfredbez alfredbez removed their assignment Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants