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

Setting whether to output the initialization log #143

Conversation

KentarouTakeda
Copy link
Contributor

Hi!

In this pull request, I propose a configurable option to selectively enable logging for the Laravel application initialization process orchestrated by Bref.

Added log_init option to the config file published with php artisan vendor:publish --tag=bref-config.

Motivation

The intention is to streamline the log output to CloudWatch by allowing selective logging, enhancing the readability and utility of the logs.

Currently, logs such as the following are output.

Creating storage directories: /tmp/storage/bootstrap/cache, /tmp/storage/framework/cache, /tmp/storage/framework/views, /tmp/storage/psysh
Running 'php artisan config:cache' to cache the Laravel configuration
INFO  Configuration cached successfully.  

These logs are useful for understanding how Bref is working within the Lambda runtime and whether the configuration of the Laravel application we have deployed is as intended.

However, this verbosity becomes superfluous when multiple cold starts occur in rapid succession, as the same messages are replicated each time.
(For example, when Bref\LaravelBridge\Queue\QueueHandler handles a large number of "Push Notifications" sent simultaneously)

This option aims to provide flexibility in managing log verbosity based on the developers' needs during different stages of deployment and operation.

Thank you.

@KentarouTakeda
Copy link
Contributor Author

This fix may solve some of the issues in #134.

The issue also mentions changes to the log output destination and format. Although these points are not supported by this Pull Request, it will prevent log output in a format other than Json from interfering with other programs.

/cc @micheleorselli

src/bref-init.php Outdated Show resolved Hide resolved
src/StorageDirectories.php Outdated Show resolved Hide resolved
src/bref-init.php Outdated Show resolved Hide resolved
@KentarouTakeda
Copy link
Contributor Author

@tillkruss @georgeboot
I think I have corrected all the points you have pointed out. Did you have any other problems?

@georgeboot
Copy link
Contributor

I like the feature as is, but personally would flip the logic and make logging opt-out using environment variables. It's now technically a breaking change and only few users will care about it.

@tillkruss
Copy link
Member

I like the feature as is, but personally would flip the logic and make logging opt-out using environment variables. It's now technically a breaking change and only few users will care about it.

Agreed.

@KentarouTakeda
Copy link
Contributor Author

Fixed in cf92542

For details such as the name, I have enabled "Allow edits and access to secrets by maintainers" setting in the Pull Request, so feel free to modify it in any way you like.

@mnapoli mnapoli merged commit 4e4432e into brefphp:master Dec 6, 2023
12 checks passed
KentarouTakeda added a commit to KentarouTakeda/example-serverless-laravel that referenced this pull request Dec 6, 2023
@KentarouTakeda KentarouTakeda deleted the setting-whether-to-output-the-initialization-log branch December 6, 2023 23:51
@KentarouTakeda
Copy link
Contributor Author

Thanks!

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.

None yet

4 participants