-
Notifications
You must be signed in to change notification settings - Fork 22
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 config to turn off sending the logs to Loki #16
base: master
Are you sure you want to change the base?
Conversation
First of all, thank you for your contribution. |
https://www.php.net/manual/en/function.curl-setopt.php Use 0 to wait indefinitely. This is the opposite of what we want to achieve. And even if it had worked I still wouldn't want the entire record formatting/processing and curl handle init part to run for no reason. |
Ah yes indeed ! I totally forgot about this "wait indefinitely" behaviour.
That being said if you do not need the loki handler in test environment,
why would you just not configure it in the test environment ? Or is it just
a CI desired behaviour ?
Truth be told I do not use the package in Laravel myself so I might be
missing some specific behaviour from Laravel configurations that prevent
you from doing so.
In Symfony if I wanted to turn it off in test I'd simply not configure the
service or, allthough I wouldn't, I could let it be configured but not be
called as a handler by monolog and it would be removed from upon
compilation of the symfony container.
Just trying to understand the exact use case here before merging your
proposal.
Le jeu. 17 févr. 2022, 21:01, Antonio Pauletich ***@***.***>
a écrit :
… https://www.php.net/manual/en/function.curl-setopt.php
https://curl.se/libcurl/c/CURLOPT_TIMEOUT_MS.html
Use 0 to wait indefinitely.
Default timeout is 0 (zero) which means it never times out during transfer.
This is the opposite of what we want to achieve. And even if it had worked
I still wouldn't want the entire record formatting/processing and curl
handle init part to run for no reason.
—
Reply to this email directly, view it on GitHub
<#16 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAU7WJOKHWUWBWXIEWIH7KDU3VHZBANCNFSM5OVQM4CA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID:
***@***.***>
|
In Laravel all config is done through PHP files which return an array. The logging config file is here -> https://github.com/laravel/laravel/blob/9.x/config/logging.php There's no concept in Laravel of environment specific config files like Symfony has (e.g. For example, in the case of the Sentry Monolog handler you just set the DSN parameter to an empty string or null and instead of an actual HTTP transport which sends stuff to Sentry you get the |
@@ -111,6 +113,7 @@ monolog: | |||
'context' => [], | |||
'labels' => [], | |||
'client_name' => '', | |||
'is_sending_enabled' => (bool) env('IS_LOKI_LOGGING_ENABLED', true), |
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.
Sorry it took me a while to get back to this.
Please rename the parameter (and the corresponding property) to enableSend
.
Also there seem to be some conflict in your PR
After configuring the logger in our Laravel app I've noticed that the test runs in the CI pipeline became a lot slower. Turns out that the package is always trying to ping Loki and since our Docker CI setup has no internet connectivity (on purpose) that slows down the tests as the handler waits until curl timeouts.
I've added a new config setting to address that so that we can disable this handler for the test environment.