-
Notifications
You must be signed in to change notification settings - Fork 120
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
Add to config minimumLevel option. #440
Conversation
Thanks @Yozhef! @ArturMoczulski is there a way to do this already? If not, this seems like a reasonable request to me. |
@Yozhef thanks for submitting the PR However, I actually can't figure out what is the feature you need here. The PR only contain adding the From what you described though, it seems you want to have conditional error levels depending on the environment or the config file loaded. Did I get that right? I'm not sure how a feature called Can you guide here what you are actually after? |
@ArturMoczulski I need this parameter on other project Rollbar company rollbar-php-symfony-bundle. On this project error level is harcode on createRollbarHandler function in RollbarHandlerFactory class. I need configure minimum error level on my yaml config. I create PR on use new config =) P.S Would be grateful if you made a revue here here too |
Ok. Now I see. Thanks for the explanation. I'll leave some feedback on the PR |
src/Config.php
Outdated
@@ -56,7 +56,8 @@ class Config | |||
'include_raw_request_body', | |||
'local_vars_dump', | |||
'max_nesting_depth', | |||
'max_items' | |||
'max_items', | |||
'minimum_level' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this option would only be used with the Monolog handler. I suggest changing this config option to monolog_minimum_level
. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think is better because this params is found and maybe people use this. You have a good test how it's work. And very long name monolog_minimum_level
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My concern is with the possible confusion of what this config option might be. This is something that will be only used with Monolog handlers. If we don't make that crystal clear a lot of users might mistakenly assume the config option will apply even if they don't use Monolog. Does it make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ArturMoczulski I thought and still reworked on your offer. =)
src/Defaults.php
Outdated
@@ -358,4 +363,5 @@ public function maxItems($value = null) | |||
private $scrubFields; | |||
private $customTruncation = null; | |||
private $maxItems = 10; | |||
private $minimumLevel = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also should probably default this to \Monolog\Logger::error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it's good =)
Thanks for those changes @Yozhef . Actually, I made a mistake. As you could see we already had the I continue the work on this in #442 and will be merging in a moment. Let's continue our conversation there. |
Hello everybody.
Please add the configuration property of the minimum error level.
My problem is:
I have 2 projects in one, I have to log errors from
Error
and in another fromInfo
. Therefore, I need the option to configure the level of theclass Config -> $options -> in my yml
required error level.