laravel-bridge icon indicating copy to clipboard operation
laravel-bridge copied to clipboard

Stack channel logging issue

Open gusiq opened this issue 5 years ago • 6 comments

Hey! I'm having an issue when using the package because of line 46 of BrefServiceProvider:

// We change Laravel's default log destination to stderr
$logDriver = Config::get('logging.default');
if ($logDriver === 'stack') {
    Config::set('logging.default', 'stderr');
}

We're using multi-channel logging, and our stack channel already include stderr. Config bellow:

'stack' => [
    'driver' => 'stack',
    'channels' => ['stderr', 'bugsnag'],
    'ignore_exceptions' => false,
],

When using the package the bugsnag just stop working.

Couldn't we just leave this to user or at least change approach to just ensure stack contains stderr instead of changing logging.default (like below example)?

// We change Laravel's default log destination to stderr
$logDriver = Config::get('logging.default');
if ($logDriver === 'stack') {
    $channels = Config::get('logging.channels.stack.channels', []);
    array_push($channels, 'stderr');
    Config::set('logging.channels.stack.channels', array_unique($channels));
}

gusiq avatar Dec 02 '20 00:12 gusiq

Hi, that's an interesting approach and I like it.

However, we need to make sure that the logger does not try to write to disk: would you solution make sure of that?

mnapoli avatar Dec 02 '20 10:12 mnapoli

Well we could filter channels to remove all that use filesystem as storage. User still could use some custom channel that writes to disk, but I think the risk is very low.

// We remove single and daily channels and ensure that stderr is used
$logDriver = Config::get('logging.default');
if ($logDriver === 'stack') {
    $channels = array_filter(Config::get('logging.channels.stack.channels', []), function ($value) {
        return !in_array($value, ['single', 'daily']);
    });
    array_push($channels, 'stderr');
    Config::set('logging.channels.stack.channels', array_unique($channels));
}

It's not a perfect solution, but I think can solve most use cases. Syslog and errorlog aren't a problem, right? What do you think?

gusiq avatar Dec 02 '20 17:12 gusiq

Right, that makes sense. However that sounds fragile to me: new versions of Laravel could very well rename single to something else for example.

I would love to find a solution that's a bit more reliable for the future, but I'm not sure what/how 🤔

mnapoli avatar Jan 15 '21 09:01 mnapoli

As one who also uses Bugsnag and is looking to use this project in production, I'd rather accept the risk of the filesystem being implemented in a Laravel upgrade where my radar is up for potential issues than sacrifice Bugsnag support. So I'd be a fan of just eliminating the logging file aliases.

bkuhl avatar Jun 02 '21 11:06 bkuhl

The variable name $logDriver is a bit misleading. The code reads from the configuration 'logging.default', which is actually the channel name.

I think the easiest solution just set the LOG_CHANNEL environment variable to be a custom one, e.g. lambda-stack, and put only stderr and bugsnag into the channels array.

        'lambda-stack' => [
            'driver' => 'stack',
            'channels' => [
                'bugsnag',
                'stderr',
            ],
            'ignore_exceptions' => false,
        ],

JonathanGuo avatar Jun 03 '21 05:06 JonathanGuo

Just got stung by this issue while trying to add Flare to the stack log channel.

One solution might be to add a new log channel named serverless to the logging config, then change the code snippet in the OP to always set the default logging channel to serverless. By default that could only include the stderr driver, but users could add whatever other drivers they want as well. The docs would need to point this out and mention that channels that expect access to a filesystem shouldn't be used.

In any case, this issue definitely needs mentioning in the docs.

zakini avatar Jun 20 '22 15:06 zakini