Skip to content

Add config to turn off sending the logs to Loki#16

Open
X-Coder264 wants to merge 1 commit intoitspire:masterfrom
X-Coder264:is-sending-enabled-config
Open

Add config to turn off sending the logs to Loki#16
X-Coder264 wants to merge 1 commit intoitspire:masterfrom
X-Coder264:is-sending-enabled-config

Conversation

@X-Coder264
Copy link
Copy Markdown

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.

@rrajkomar
Copy link
Copy Markdown
Contributor

First of all, thank you for your contribution.
That is indeed something that happens when the Loki server is unavailable and/or unreachable.
However I do believe that you can already bypass this by overrriding the curl options (see the curl_options setting which was added by @cstuder) with CURLOPT_TIMEOUT_MS set to 0.
Did you try this settting ? Please try and get back to me.
In the meantime I'll leave this PR opened.

@X-Coder264
Copy link
Copy Markdown
Author

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.

@rrajkomar
Copy link
Copy Markdown
Contributor

rrajkomar commented Feb 17, 2022 via email

@X-Coder264
Copy link
Copy Markdown
Author

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. prod/monolog.yaml and test/monolog.yaml). Theoretically I could write some ugly if statements and then merge or not merge the logger into the returned array, but nobody does that in the Laravel ecosystem as all log handlers that I've worked with have a config option with which you can turn them off via an env variable so that the returned array in that logging.php file does not depend on the environment.

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 NullTransport which does nothing -> https://github.com/getsentry/sentry-php/blob/3.3.7/src/Transport/DefaultTransportFactory.php#L61-L63

'context' => [],
'labels' => [],
'client_name' => '',
'is_sending_enabled' => (bool) env('IS_LOKI_LOGGING_ENABLED', true),
Copy link
Copy Markdown
Contributor

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

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.

2 participants