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

Stack channel logging issue #27

Open
gusiq opened this issue Dec 2, 2020 · 6 comments
Open

Stack channel logging issue #27

gusiq opened this issue Dec 2, 2020 · 6 comments
Labels
enhancement New feature or request

Comments

@gusiq
Copy link

gusiq commented Dec 2, 2020

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));
}
@mnapoli
Copy link
Member

mnapoli commented Dec 2, 2020

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?

@gusiq
Copy link
Author

gusiq commented Dec 2, 2020

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?

@mnapoli mnapoli added the enhancement New feature or request label Jan 15, 2021
@mnapoli
Copy link
Member

mnapoli commented Jan 15, 2021

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 🤔

@bkuhl
Copy link

bkuhl commented Jun 2, 2021

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.

@JonathanGuo
Copy link

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,
        ],

@zakini
Copy link

zakini commented Jun 20, 2022

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.

mnapoli pushed a commit that referenced this issue Mar 9, 2023
* Ensure valid handle to stderr

* Update runtime.php

Co-authored-by: Till Krüss <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants